-
Notifications
You must be signed in to change notification settings - Fork 4.9k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Regenerating SDK based on swagger changes on List/Cancel operation for Azure database and Elastic pool #4106
Conversation
… list database/pool operation
Can one of the admins verify this patch? |
3 similar comments
Can one of the admins verify this patch? |
Can one of the admins verify this patch? |
Can one of the admins verify this patch? |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
- Code needs to be generated from the swagger spec checked into master
- Package Version needs to be updated
GitHub user: kosta-bizetic | ||
Branch: master | ||
Commit: fb9e1b2a7561e7bd7d697011bd063964f2521861 | ||
GitHub user: payiAzure |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please pull down latest changes and run generate.cmd
again, this should log the commit id here
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Also, looks like you are generating code from your local branch, please generate the code from the checked in swagger
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@dsgouda Sync with the checked-in swagger and regenerate the sdk from master in the second commit. Please take a look
@azuresdkci test this please |
…ecked-in changes and renenerate the SDK
Restarting CI |
@azuresdkci rebuild |
@azuresdkci retest |
Please do the last 2 items from the checklist:
:) |
csproj & AssemblyInfo.cs should have minor version bump (e.g. 1.12.0.0 -> 1.13.0.0) and please also edit release notes in csproj (remove old release notes describing 1.12 and add new notes describing 1.13) |
…-specs. Bump the api version in Assembly file and csproj
@jaredmoo Update the pull request, address comments on:
|
Dtu = 250, | ||
Tags = tags | ||
}); | ||
Thread.Sleep(TimeSpan.FromSeconds(5)); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We should only sleep when in record mode. Take a look at other tests e.g. Restore tests
// No need to sleep if we are playing back the recording.
if (HttpMockServer.Mode == HttpRecorderMode.Record)
{
Thread.Sleep(TimeSpan.FromSeconds(5));
}
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
btw how reliable is this test? Does it generally pass every time?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
yes, when I run the test for multiple time, it passed every time.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@jaredmoo update the test in the new commit of the PR: only sleep when in Record mode
@@ -1,11 +1,13 @@ | |||
2018-01-31 10:51:15 UTC | |||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please pull down the latest changes from upstream and run the generate.cmd
command again, this should log the commit id here.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@dsgouda . I did, but the interesting thing is no commit id shown in the file. I tried several times.
my forked azurePayi/azure-sdk-for-net repo is updated with the official azure/azure-sdk-for-net repo, and I run the generate.cmd against official azure/azure-rest-spi-specs
since my swagger changes already were merged to the official azure-rest-spi-specs
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@dsgouda Thanks for your help! Update the PR which has commit id in the metadata file.
@dsgouda Hi Deepak, can we get this PR review complete asap and merge? The sdk-for-phyton publish has dependency on this |
@payiAzure Please fix the build issues here |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM subject to CI passing
@azuresdkci ReTest this |
@azuresdkci retest this please |
… list database/pool operation
Description
This pull request is to regenerate SDK based on the swagger changes on List/Cancel Operation for Azure database and elastic pool.
The related swagger pull request that has been merged in the azure-rest-api-specs are:
Azure/azure-rest-api-specs#2520
Azure/azure-rest-api-specs#2592 (only update the sql-resourcemanager readme)
Azure/azure-rest-api-specs#2640 (refactor sql readme. List/Cancel operation is included in the refactored sql readme version)
[Do no merge] notice in the tile: we're doing the ARM manifest update on production currently for new version API. Will remove it from title and do the merge after the ARM manifest update is done.
This checklist is used to make sure that common guidelines for a pull request are followed.
General Guidelines
Testing Guidelines
SDK Generation Guidelines
*.csproj
andAssemblyInfo.cs
files have been updated with the new version of the SDK.